Skip to content

Comments

feat/components/header: added header with testUser and onClick functi…#21

Open
Pawel-dev5 wants to merge 2 commits intomasterfrom
feat/header
Open

feat/components/header: added header with testUser and onClick functi…#21
Pawel-dev5 wants to merge 2 commits intomasterfrom
feat/header

Conversation

@Pawel-dev5
Copy link
Collaborator

feat/components/header: added header with testUser and onClick function to change logIn/logOut status

Comment on lines 15 to 27
function changeLogin() {
if (users.isLogged === false) {
setUsers((prevState) => ({
...prevState,
isLogged: true,
}));
} else {
setUsers((prevState) => ({
...prevState,
isLogged: false,
}));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice destructuring! Here's shorter version though 😄

Suggested change
function changeLogin() {
if (users.isLogged === false) {
setUsers((prevState) => ({
...prevState,
isLogged: true,
}));
} else {
setUsers((prevState) => ({
...prevState,
isLogged: false,
}));
}
}
const toggleIsUserLogged = () => {
setUsers((prevState) => ({
...prevState,
isLogged: !prevState.isLogged,
}));
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dzięki, zmieniłem zgodnie z sugestią :)

};

export default function Header() {
const [users, setUsers] = useState(testUser);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

users (plural) implies that there's multiple users. Use just user, setUser

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, zmienione

<Typography.Nav>
<li>Explore</li>
{users.isLogged ? (
<li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using native html elements, create styled-component for that styled.li. That way you don't need to style li in Nav. Just style it independently

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utworzona lista

return (
<DefaultLayout gapRow={3} marginVertical={5} marginHorizontal="auto">
<DefaultLayout gapRow={3} marginHorizontal="auto">
<Header />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since header will always be present, I think importing it in src/index.tsx will be better. We want to display header everywhere not only in / (home route)

@marcinbar7
Copy link
Collaborator

@Pawel-dev5 can you update the branch please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants